Allow VolumeAttributesClassName changes in storageConfig webhook#921
Allow VolumeAttributesClassName changes in storageConfig webhook#921SreedevT wants to merge 1 commit intok8ssandra:masterfrom
Conversation
8f3e45a to
588715d
Compare
| oldClaimSpec.Resources.Requests = newClaimSpec.Resources.Requests | ||
| } | ||
|
|
||
| // VolumeAttributesClassName changes are always allowed as they represent in-place volume |
There was a problem hiding this comment.
This isn't strictly true, the StorageClass must support ModifyVolume. Also, did you verify if the StatefulSet controller accepts these changes and applies them?
There is also no monitoring in here that the VolumeAttributesClass change has been approved as the CSI driver can reject it. That would require changes like the resizing parameter checks and correct reconciliation until the change has been approved.
This is relevant because in some drivers such as AWS, one can change from gp2 to gp3 and this operation might not be instant and as such will require the operator to correctly wait for the changes to be applied. There are also restrictions on how many times per day such operations can be done and we can't set the datacenter to a ready state if the changes haven't been applied (or for example incorrect / outdated parameters are used).
There was a problem hiding this comment.
Thats fair, I have made the PR a draft. Current PR only bypasses the validation and does not make any changes.
Can I make it a gated feature with more checks in place?
Where is this change and in which scenarios can it happen?
They only GAed in 1.34 and marked as stable in upcoming 1.36. |
588715d to
6604e5f
Compare
VolumeAttributesClassName (VAC) is used for in-place volume performance class changes (e.g. AWS EBS VolumeAttributesClass) and does not replace the PVC. The webhook was blocking all changes to CassandraDataVolumeClaimSpec beyond storage size, which prevented legitimate VAC updates. Normalize VolumeAttributesClassName before the DeepEqual check so changes to it are always permitted. Also consolidates the two separate nil guards for CassandraDataVolumeClaimSpec into one block, fixing a latent nil panic if AllowStorageChangesAnnotation was set on a DC with no StorageConfig.
6604e5f to
e16ff5b
Compare
What this PR does:
VolumeAttributesClassName (VAC) is a field on
PersistentVolumeClaimSpecused for in-place volume performance class changes (e.g. AWS EBS VolumeAttributesClass). It does not replace the PVC — only its performance tier is updated. The webhook was blocking all changes toCassandraDataVolumeClaimSpecbeyond storage size, which prevented legitimate VAC updates with an error like:This PR normalizes
VolumeAttributesClassNamebefore theDeepEqualcheck so changes to it are always permitted. It also consolidates the two separate nil guards forCassandraDataVolumeClaimSpecinto one block, fixing a latent nil panic ifAllowStorageChangesAnnotationwas set on a DC with noStorageConfig.Which issue(s) this PR fixes:
Fixes #922
Checklist